Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: block validation #1873

Merged
merged 3 commits into from
Nov 21, 2023
Merged

Refactor: block validation #1873

merged 3 commits into from
Nov 21, 2023

Conversation

xDimon
Copy link
Member

@xDimon xDimon commented Nov 21, 2023

Referenced issues

Needed for #1591 (as part of preparation for sassafras block validation develop)

Description of the Change

Makes block validation consensus agnostic as more as possible: bock validation will be called from consensus.
This is needed to make validation block internally particular consensus

Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
return consensus;
}
}
BOOST_UNREACHABLE_RETURN({});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to put some SL_CRITICAL message before the macro?

// In order to ease possible debugging after the later changes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It really unreachable. At least one (last) consensus will be returned as fallback. Empty list throws exception (assert) in ctor


// TODO: Code for trying to select another consensus
}
BOOST_UNREACHABLE_RETURN({});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest covering such cases with log messages, since the code may change and the line might got reached.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely I'll remove selector for finality consensus.

test/core/consensus/babe/babe_block_validator_test.cpp Outdated Show resolved Hide resolved
using testing::Mock;
using testing::Return;

class BabeBlockValidatorTest : public testing::Test {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

Signed-off-by: Dmitriy Khaustov aka xDimon <khaustov.dm@gmail.com>
@xDimon xDimon enabled auto-merge (squash) November 21, 2023 13:13
@xDimon xDimon merged commit a770c74 into master Nov 21, 2023
9 of 10 checks passed
@xDimon xDimon deleted the refactor/block_validating branch November 21, 2023 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants